Skip to content

Release 3.1.2: SDID scale fix, convergence warnings, roadmap refresh#316

Merged
igerber merged 2 commits intomainfrom
release/3.1.2
Apr 18, 2026
Merged

Release 3.1.2: SDID scale fix, convergence warnings, roadmap refresh#316
igerber merged 2 commits intomainfrom
release/3.1.2

Conversation

@igerber
Copy link
Copy Markdown
Owner

@igerber igerber commented Apr 18, 2026

Summary

Methodology references

  • Method name(s): N/A — this release carries only numerical-stability / convergence-signaling fixes and version metadata. The underlying SyntheticDiD / ImputationDiD / TwoStageDiD / within_transform algorithms are unchanged; only floating-point conditioning and convergence-warning paths are touched.
  • Paper / source link(s): N/A (no methodology change).
  • Any intentional deviations from the source: None.

Validation

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

Generated with Claude Code

… remove deprecated SyntheticDiD params

Package four merged PRs (#312 SDID catastrophic cancellation at extreme Y scale,
#313 roadmap refresh, #314 FE imputation non-convergence signaling, #315 Frank-Wolfe
SC weight solver non-convergence signaling) as 3.1.2. Also remove the
SyntheticDiD(lambda_reg=...) and SyntheticDiD(zeta=...) kwargs, which have been
deprecated with DeprecationWarning since v2.3.1 (2026-02-10) in favor of
zeta_omega / zeta_lambda; their warning messages announced removal in v3.1. Passing
the old kwargs now raises TypeError at __init__ and ValueError: Unknown parameter
at set_params. Internal ridge-regression helpers that accept a lambda_reg parameter
(compute_synthetic_weights, rank_control_units, Rust FFI bindings) are unaffected.

Version strings bumped in diff_diff/__init__.py, pyproject.toml, rust/Cargo.toml,
and diff_diff/guides/llms-full.txt. CHANGELOG populated with Fixed / Changed /
Removed sections and comparison-link footer. TODO.md's "Deprecated Code" entry
removed now that the task is done.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

Overall Assessment

⚠️ Needs changes

Executive Summary

  • The SyntheticDiD estimator, regularization, weighting, and placebo/bootstrap/jackknife inference code are unchanged; I found no methodology mismatch against the registry or in-code reference to Arkhangelsky et al.
  • P1: this PR removes public SyntheticDiD kwargs while simultaneously releasing 3.1.2, which contradicts the repo’s stated Semantic Versioning contract and can break existing 3.1.1 caller code on a patch upgrade.
  • I found no new NaN/Inf inference anti-patterns, control-group logic changes, or parameter-propagation gaps in the modified SDID code.
  • No performance or security issues are introduced by the diff.
  • I could not run the repo tests in this sandbox because pytest and numpy are not installed.

Methodology

  • No findings. The diff only removes deprecated aliases from SyntheticDiD.__init__ and set_params; the estimator definition, auto-regularization, Frank-Wolfe weighting, and placebo/bootstrap/jackknife SE behavior still match the SyntheticDiD registry entry and the class docstring/reference to Arkhangelsky et al. docs/methodology/REGISTRY.md:L1391-L1545 diff_diff/synthetic_did.py:L26-L167

Code Quality

  • No findings.

Performance

  • No findings.

Maintainability

  • Severity: P1. Impact: this PR makes SyntheticDiD(lambda_reg=...) and SyntheticDiD(zeta=...) hard errors by removing them from the constructor signature and by letting set_params() reject them as unknown, but the same PR versions the package as 3.1.2 even though the changelog says the project adheres to Semantic Versioning. Existing 3.1.1 caller code can therefore fail immediately on what appears to be a safe patch upgrade. Prior deprecation warnings do not mitigate that release-contract break under SemVer. Concrete fix: either keep the deprecated aliases warning-only throughout the 3.x line, or change this release to the next major version and update all version declarations and release notes consistently. Evidence: CHANGELOG.md:L5-L21 pyproject.toml:L5-L8 diff_diff/init.py:L234-L234 diff_diff/synthetic_did.py:L136-L145 diff_diff/synthetic_did.py:L1447-L1453

Tech Debt

  • No findings. Removing the completed deprecated-code TODO is consistent with the code change; I did not see new deferred-work items that need tracking.

Security

  • No findings.

Documentation/Tests

  • No findings in adjacent user docs; I did not find current SyntheticDiD examples in README.md, docs/, or diff_diff/guides that still advertise the removed aliases. The remaining suite still covers generic unknown-constructor and unknown-parameter rejection paths, although the exact deprecated names are no longer exercised directly. Review note: I could not execute the tests here because the sandbox lacks pytest and numpy. Evidence: tests/test_estimators.py:L3075-L3078 tests/test_methodology_sdid.py:L1067-L1071

Path to Approval

  1. Choose a compatible release contract. If this remains 3.1.2, restore lambda_reg and zeta handling in SyntheticDiD.__init__ and set_params() with deprecation warnings. If the hard removal is intentional, bump the release to the next major version everywhere it is declared.
  2. Add an explicit regression test for the chosen contract using the deprecated names in both constructor and set_params(), so future refactors cannot silently change the behavior again.

Per CI review feedback (#316): removing public kwargs under a patch version
violates Semantic Versioning, which CHANGELOG.md explicitly claims to adhere to.
Restore lambda_reg and zeta handling in SyntheticDiD.__init__ and set_params
as warning-only, and bump the removal target in the DeprecationWarning text
from "v3.1" to "v4.0.0". The 3.1.2 release now carries only the four fix/doc
PRs (#312 SDID scale, #313 roadmap, #314 FE imputation convergence, #315
Frank-Wolfe convergence) with no breaking changes.

- diff_diff/synthetic_did.py: restore deprecated kwargs + warnings (v4.0.0 text)
- tests/test_methodology_sdid.py: restore TestDeprecatedParams class + set_params deprecation test
- tests/test_estimators.py: restore test_deprecated_params
- CHANGELOG.md: drop Removed section; add Changed entry documenting the v3.1 -> v4.0.0 bump in the removal target
- TODO.md: restore Deprecated Code section with v4.0.0 removal target and SemVer rationale

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@igerber
Copy link
Copy Markdown
Owner Author

igerber commented Apr 18, 2026

/ai-review

@igerber igerber changed the title Release 3.1.2 + remove deprecated SyntheticDiD lambda_reg/zeta Release 3.1.2: SDID scale fix, convergence warnings, roadmap refresh Apr 18, 2026
@github-actions
Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: 9230c8f483827996f8cdabe8fced17a054a1a768


Overall Assessment

✅ Looks good

Executive Summary

Methodology

Code Quality

Performance

  • No findings. No estimator, solver, or numerical hot-path code changed in this re-review diff.

Maintainability

Tech Debt

  • No findings. The deferred alias removal is explicitly tracked in TODO.md, so this PR does not introduce untracked debt. TODO.md:L123-L127

Security

  • No findings. The diff is limited to version strings, release notes, TODO text, and warning messages.

Documentation/Tests

@igerber igerber added the ready-for-ci Triggers CI test workflows label Apr 18, 2026
@igerber igerber merged commit ba790b0 into main Apr 18, 2026
23 of 24 checks passed
@igerber igerber deleted the release/3.1.2 branch April 18, 2026 17:57
igerber added a commit that referenced this pull request Apr 18, 2026
CI review on #322 flagged that the 3.1.3 entries for PR #319 (sparse->dense
lstsq fallback) and PR #317 (TROP non-convergence) claimed ConvergenceWarning,
but the actual implementations emit UserWarning (imputation.py, two_stage.py,
utils.py, trop.py, and the REGISTRY.md contract all use UserWarning). Users
filtering warnings by category would be misled.

Same factual error was in the 3.1.2 entries I wrote in PR #316 for PR #314
and PR #315. Fixing both entries in this PR — CHANGELOG is a living doc and
the warning-category drift is actionable-ly misleading.

No code or test changes; CHANGELOG-only edit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-ci Triggers CI test workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant